-
-
Notifications
You must be signed in to change notification settings - Fork 9
fix: offline map rendering after extended offline period (#354) #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Phase 02.2: Offline Map Tile Rendering - Identified root cause: HTTP style cache expiration after extended offline - Current code uses same HTTP URL for online/offline, relies on MapLibre cache - MapLibre respects HTTP cache headers, style JSON expires after days - Solution: Generate and cache local style JSON during download - Use Style.Builder().fromJson() for offline, not fromUri() - Requires database schema update for localStylePath storage - HIGH confidence on all findings
Phase 02.2: Offline Map Tile Rendering - 2 plan(s) in 2 wave(s) - 1 parallel (wave 1), 1 sequential (wave 2) - Ready for execution
- Add explicit repository implementation for getFirstCompletedRegionWithStyle() wrapping offlineMapRegionDao.getFirstCompletedRegionWithLocalStyle()?.toOfflineMapRegion() - Reframe must_haves.truths from implementation-focused to user-observable outcomes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 2.2 inserted: Offline Map Tile Rendering (#354) - Updated current focus to Phase 2.2 - Added roadmap evolution entry with root cause summary - Updated next step pointer Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add localStylePath column to OfflineMapRegionEntity (nullable String) - Create database migration 34->35 adding localStylePath column - Add DAO methods: updateLocalStylePath() and getFirstCompletedRegionWithLocalStyle() - Add repository methods wrapping DAO: updateLocalStylePath() and getFirstCompletedRegionWithStyle() - Update OfflineMapRegion domain model with localStylePath field - Add localStylePath to entity->domain mapping in toOfflineMapRegion() This enables storing the path to locally cached style JSON files for offline map rendering. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add fetchAndCacheStyleJson() method to OfflineMapDownloadViewModel
- Fetch style JSON from MapTileSourceManager.DEFAULT_STYLE_URL after download completes
- Save style JSON to filesDir/offline_styles/{regionId}.json
- Persist local file path to database via updateLocalStylePath()
- Call fetchAndCacheStyleJson() in onComplete callback after markCompleteWithMaplibreId()
- Add withContext import for IO dispatcher context switching
- Style caching failure is non-fatal: logged as warning, download still marked complete
This ensures offline maps have persistent style JSON that survives HTTP cache expiration.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move fetchAndCacheStyleJson() to separate coroutine (non-blocking) - Add 5-second timeout to URL fetch to prevent infinite hangs - Update state to isComplete before launching style fetch - Add mock for updateLocalStylePath in OfflineMapDownloadViewModelTest - Update test comment to reflect non-blocking behavior This ensures the download completion UI update happens immediately while style caching happens asynchronously in the background without blocking. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tasks completed: 2/2 - Add localStylePath to database schema and data layer - Fetch and cache style JSON during offline map download SUMMARY: .planning/phases/02.2-offline-map-tile-rendering/02.2-01-SUMMARY.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…olution - Add MapStyleResult.OfflineWithLocalStyle variant with localStylePath - Check for local style JSON in getMapStyle() hasOffline branch - Verify file existence before using local style - Fallback to HTTP URL if no local style exists (backward compatibility)
- Add OfflineWithLocalStyle handler to both when(styleResult) blocks - Load style JSON from local file using Style.Builder().fromJson() - Network connectivity automatically disabled (allowNetwork = false) - Both initial style loading and style updates now support local style JSON
Tasks completed: 3/3 - Add OfflineWithLocalStyle variant and update style resolution - Handle OfflineWithLocalStyle in MapScreen - Human verification checkpoint (approved) SUMMARY: .planning/phases/02.2-offline-map-tile-rendering/02.2-02-SUMMARY.md Phase 02.2 (Offline Map Tile Rendering) is now COMPLETE. Issue #354 resolved: offline maps render full detail indefinitely.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile OverviewGreptile SummaryFixes offline map rendering after extended offline periods by caching style JSON locally during download and loading it via What Changed:
Strengths:
Issue (already flagged):
Confidence Score: 4.5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant MapScreen
participant MapTileSourceManager
participant OfflineMapRegionRepository
participant Database
participant FileSystem
Note over User,FileSystem: Write Path: During Offline Map Download
User->>OfflineMapDownloadViewModel: Download offline map region
OfflineMapDownloadViewModel->>MapLibre: Download tiles
MapLibre-->>OfflineMapDownloadViewModel: Download complete
OfflineMapDownloadViewModel->>Database: markCompleteWithMaplibreId()
OfflineMapDownloadViewModel->>OfflineMapDownloadViewModel: fetchAndCacheStyleJson() (async)
OfflineMapDownloadViewModel->>FileSystem: URL.readText() style JSON (5s timeout)
FileSystem-->>OfflineMapDownloadViewModel: Style JSON content
OfflineMapDownloadViewModel->>FileSystem: Write to filesDir/offline_styles/{regionId}.json
OfflineMapDownloadViewModel->>Database: updateLocalStylePath()
Note over User,FileSystem: Read Path: Viewing Map After Extended Offline Period
User->>MapScreen: View map (offline, HTTP cache expired)
MapScreen->>MapTileSourceManager: getMapStyle()
MapTileSourceManager->>OfflineMapRegionRepository: getFirstCompletedRegionWithStyle()
OfflineMapRegionRepository->>Database: Query for completed region with localStylePath
Database-->>OfflineMapRegionRepository: OfflineMapRegion with localStylePath
OfflineMapRegionRepository-->>MapTileSourceManager: Region with style path
MapTileSourceManager->>FileSystem: Check if file exists
FileSystem-->>MapTileSourceManager: File exists
MapTileSourceManager-->>MapScreen: MapStyleResult.OfflineWithLocalStyle(path)
MapScreen->>FileSystem: File(path).readText() in try-catch
FileSystem-->>MapScreen: Style JSON content
MapScreen->>MapLibre: Style.Builder().fromJson(styleJson)
MapLibre-->>User: Map renders with full vector tile detail
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 3 comments
| is MapStyleResult.OfflineWithLocalStyle -> { | ||
| val styleJson = java.io.File(styleResult.localStylePath).readText() | ||
| Style.Builder().fromJson(styleJson) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing try-catch around File.readText() - if file is corrupted or deleted, app will crash
| is MapStyleResult.OfflineWithLocalStyle -> { | |
| val styleJson = java.io.File(styleResult.localStylePath).readText() | |
| Style.Builder().fromJson(styleJson) | |
| } | |
| is MapStyleResult.OfflineWithLocalStyle -> { | |
| try { | |
| val styleJson = java.io.File(styleResult.localStylePath).readText() | |
| Style.Builder().fromJson(styleJson) | |
| } catch (e: Exception) { | |
| Log.e("MapScreen", "Failed to read cached style JSON, falling back to HTTP", e) | |
| Style.Builder().fromUri(MapTileSourceManager.DEFAULT_STYLE_URL) | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/ui/screens/MapScreen.kt
Line: 335:338
Comment:
missing try-catch around `File.readText()` - if file is corrupted or deleted, app will crash
```suggestion
is MapStyleResult.OfflineWithLocalStyle -> {
try {
val styleJson = java.io.File(styleResult.localStylePath).readText()
Style.Builder().fromJson(styleJson)
} catch (e: Exception) {
Log.e("MapScreen", "Failed to read cached style JSON, falling back to HTTP", e)
Style.Builder().fromUri(MapTileSourceManager.DEFAULT_STYLE_URL)
}
}
```
How can I resolve this? If you propose a fix, please make it concise.| is MapStyleResult.OfflineWithLocalStyle -> { | ||
| val styleJson = java.io.File(styleResult.localStylePath).readText() | ||
| Style.Builder().fromJson(styleJson) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing try-catch around File.readText() - same issue as line 335-338
| is MapStyleResult.OfflineWithLocalStyle -> { | |
| val styleJson = java.io.File(styleResult.localStylePath).readText() | |
| Style.Builder().fromJson(styleJson) | |
| } | |
| is MapStyleResult.OfflineWithLocalStyle -> { | |
| try { | |
| val styleJson = java.io.File(styleResult.localStylePath).readText() | |
| Style.Builder().fromJson(styleJson) | |
| } catch (e: Exception) { | |
| Log.e("MapScreen", "Failed to read cached style JSON, falling back to HTTP", e) | |
| Style.Builder().fromUri(MapTileSourceManager.DEFAULT_STYLE_URL) | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/ui/screens/MapScreen.kt
Line: 429:432
Comment:
missing try-catch around `File.readText()` - same issue as line 335-338
```suggestion
is MapStyleResult.OfflineWithLocalStyle -> {
try {
val styleJson = java.io.File(styleResult.localStylePath).readText()
Style.Builder().fromJson(styleJson)
} catch (e: Exception) {
Log.e("MapScreen", "Failed to read cached style JSON, falling back to HTTP", e)
Style.Builder().fromUri(MapTileSourceManager.DEFAULT_STYLE_URL)
}
}
```
How can I resolve this? If you propose a fix, please make it concise.| val styleJson = | ||
| kotlinx.coroutines.withTimeout(5000) { | ||
| java.net.URL(MapTileSourceManager.DEFAULT_STYLE_URL).readText() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
java.net.URL.readText() blocks the IO dispatcher - for large style JSONs, this could cause ANRs. Consider using OkHttp or similar for proper async I/O.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt
Line: 697:700
Comment:
`java.net.URL.readText()` blocks the IO dispatcher - for large style JSONs, this could cause ANRs. Consider using OkHttp or similar for proper async I/O.
How can I resolve this? If you propose a fix, please make it concise.Prevents crash if cached style file is corrupted or deleted between the existence check in getMapStyle() and readText() in MapScreen. Falls back to HTTP style URL on failure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The app module has sentry/noSentry product flavors, so testDebugUnitTest no longer matches any app task. Add testNoSentryDebugUnitTest to run the ~5,398 app-level unit tests that were silently skipped. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Style.Builder().fromJson()when offline, instead of relying on MapLibre's HTTP cache which expires after days.Changes
DB Schema (Plan 01 — write path)
localStylePathcolumn tooffline_map_regionsupdateLocalStylePath(),getFirstCompletedRegionWithLocalStyle()updateLocalStylePath(),getFirstCompletedRegionWithStyle()fetchAndCacheStyleJson()in OfflineMapDownloadViewModel — fetches style JSON after download completes, saves tofilesDir/offline_styles/{regionId}.json, non-blocking with 5s timeoutStyle Loading (Plan 02 — read path)
MapStyleResult.OfflineWithLocalStylesealed class variantgetMapStyle()checks for cached style file before falling back to HTTP URLwhen(styleResult)blocks in MapScreen handleOfflineWithLocalStyleviafromJson()Backward Compatibility
Test plan
Closes #354
🤖 Generated with Claude Code